-
Notifications
You must be signed in to change notification settings - Fork 409
feat: add support for custom client notifications #556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This introduces a new feature to Codex when it operates as an MCP
_client_ where if an MCP _server_ replies that it has an entry named
`"codex/sandbox-state"` in its _server capabilities_, then Codex will
send it an MCP notification with the following structure:
```json
{
"method": "codex/sandbox-state/update",
"params": {
"sandboxPolicy": {
"type": "workspace-write",
"network-access": false,
"exclude-tmpdir-env-var": false
"exclude-slash-tmp": false
},
"codexLinuxSandboxExe": null,
"sandboxCwd": "/Users/mbolin/code/codex2"
}
}
```
or with whatever values are appropriate for the initial `sandboxPolicy`.
**NOTE:** Codex _should_ continue to send the MCP server notifications
of the same format if these things change over the lifetime of the
thread, but that isn't wired up yet.
The result is that `shell-tool-mcp` can consume these values so that
when it calls `codex_core::exec::process_exec_tool_call()` in
`codex-rs/exec-server/src/posix/escalate_server.rs`, it is now sure to
call it with the correct values (whereas previously we relied on
hardcoded values).
While I would argue this is a supported use case within the MCP
protocol, the `rmcp` crate that we are using today does not support
custom notifications. As such, I had to patch it and I submitted it for
review, so hopefully it will be accepted in some form:
modelcontextprotocol/rust-sdk#556
To test out this change from end-to-end:
- I ran `cargo build` in `~/code/codex2/codex-rs/exec-server`
- I built the fork of Bash in `~/code/bash/bash`
- I added the following to my `~/.codex/config.toml`:
```toml
# Use with `codex --disable shell_tool`.
[mcp_servers.execshell]
args = ["--bash", "/Users/mbolin/code/bash/bash"]
command = "/Users/mbolin/code/codex2/codex-rs/target/debug/codex-exec-mcp-server"
```
- From `~/code/codex2/codex-rs`, I ran `just codex --disable shell_tool`
- When the TUI started up, I verified that the sandbox mode is
`workspace-write`
- I ran `/mcp` to verify that the shell tool from the MCP is there:
<img width="1387" height="1400" alt="image"
src="https://github.com/user-attachments/assets/1a8addcc-5005-4e16-b59f-95cfd06fd4ab"
/>
- Then I asked it:
> what is the output of `gh issue list`
because this should be auto-approved with our existing dummy policy:
https://github.com/openai/codex/blob/af63e6eccc35783f1bf4dca3c61adb090efb6b8a/codex-rs/exec-server/src/posix.rs#L157-L164
And it worked:
<img width="1387" height="1400" alt="image"
src="https://github.com/user-attachments/assets/7568d2f7-80da-4d68-86d0-c265a6f5e6c1"
/>
alexhancock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does leave me with a question why we wouldn't generalize it to also handle server -> client. What do you think?
I'll merge for now and we can consider.
|
@bolinfest Mind taking a look at the failing checks? Thanks |
|
@alexhancock I believe I fixed the test and clippy issues, and I think the code coverage job failed due to the test failure, so could you please let CI run again? |
Yes, I agree this should also be supported. I'm happy to help with that, but I would like to move Codex off my custom branch of |
MCP servers, particularly ones that offer "experimental" capabilities, may wish to handle custom client notifications that are not part of the standard MCP specification. This change introduces a new `CustomClientNotification` type that allows a server to process such custom notifications. - introduces `CustomClientNotification` to carry arbitrary methods/params while still preserving meta/extensions; wires it into the `ClientNotification` union and `serde` so `params` can be decoded with `params_as` - allows server handlers to receive custom notifications via a new `on_custom_notification` hook - adds integration coverage that sends a custom client notification end-to-end and asserts the server sees the method and payload Test: ```shell cargo test -p rmcp --features client test_custom_client_notification_reaches_server ```
…ifications (#7462) In #7112, I updated our `rmcp` dependency to point to a personal fork while I tried to upstream my proposed change. Now that modelcontextprotocol/rust-sdk#556 has been upstreamed and included in the `0.10.0` release of the crate, we can go back to using the mainline release.
MCP servers, particularly ones that offer "experimental" capabilities,
may wish to handle custom client notifications that are not part of the
standard MCP specification. This change introduces a new
CustomClientNotificationtype that allows a server to processsuch custom notifications.
CustomClientNotificationto carry arbitrary methods/params whilestill preserving meta/extensions; wires it into the
ClientNotificationunionand
serdesoparamscan be decoded withparams_ason_custom_notificationhookand asserts the server sees the method and payload
Test:
cargo test -p rmcp --features client test_custom_client_notification_reaches_server